-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Remove paper support #3639
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Remove paper support #3639
Conversation
Beautiful ❤️ |
I think this should be targeting the |
da63110
to
31a9fdd
Compare
31a9fdd
to
4730f8a
Compare
…m/software-mansion/react-native-gesture-handler into @latekvo/remove-paper-support-new
def isNewArchitectureEnabled() { | ||
// This is a workaround for linter crashing when applying the `com.facebook.react` plugin. | ||
return project.hasProperty("newArchEnabled") && project.newArchEnabled == "true" | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, I couldn't find any proper solution for the linter CI crashing, other than keeping this function in place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a thought, but what do you think about throwing error if new architecture is not enabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could have some environment variable for the CI to set when it wants to exclude the react plugin from the gradle build file.
what do you think about throwing error
Not sure if that's necessary, I think React Native already throws a warning when new arch is set to false on the versions where it can't be disabled.
packages/react-native-gesture-handler/apple/RNGestureHandlerDetector.mm
Outdated
Show resolved
Hide resolved
packages/react-native-gesture-handler/apple/RNGestureHandlerDetector.mm
Outdated
Show resolved
Hide resolved
def isNewArchitectureEnabled() { | ||
// This is a workaround for linter crashing when applying the `com.facebook.react` plugin. | ||
return project.hasProperty("newArchEnabled") && project.newArchEnabled == "true" | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a thought, but what do you think about throwing error if new architecture is not enabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
Description
This PR removes all paper support.
Tested on all platforms, all
basic-example
,expo-example
andmacos-example
build and run successfully.Fixme:
yarn lint:android
does not work anymoreTest plan
expo-example
andbasic-example
apps on:Android
,iOS
andMacOS